-
Notifications
You must be signed in to change notification settings - Fork 163
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
perf: slight optimization on merge states #313
Conversation
The commit msg is interesting :P |
never mind :) |
|
||
vec_t<float, vec_size> v_merged_vec; | ||
v_merged_vec.fill(0.f); | ||
if (num_index_sets > 1) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wouldn't it be cleaner to just early return instead of indenting everything?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes you are right, thanks for your suggestion.
90e5473
to
cf7a7d4
Compare
cf7a7d4
to
a8bc999
Compare
🤖 I have created a release *beep* *boop* --- ## [0.1.2](v0.1.1...v0.1.2) (2024-07-29) ### Bugfix * Fix the sampling kernel bug for cu118 ([#386](#386), [#387](#387)) ([0cd499](0cd4994), [dc3f18](dc3f184)) ### Features * add llama 3.1 style rope ([#401](#401)) ([4c89dec](4c89dec)) * non-inplace rope operators ([#405](#405)) ([74ffba1](74ffba1)) * sliding window attention ([#406](#406)) ([28cffd3](28cffd3)) * support non-contiguous (packed) input for prefill kernels ([#404](#404)) ([68c3719](68c3719)) ### Performance Improvements * slight optimization on merge states ([#313](#313)) ([701c813](701c813)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Zihao Ye <expye@outlook.com>
When cudagraph is enabled, we will still call merge states kernels for short sequence length, which incurs some unnecessary overhead.
This PR accelerates merge states kernel when there is nothing to merge (
num_index_sets=1
).We can actually write through to the target buffer for small sequence length, but I'm always lazy evaluated and I'll leave it for a future PR (if necessary).